-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mdr upgrade scenario #8699
base: master
Are you sure you want to change the base?
Mdr upgrade scenario #8699
Conversation
b40218f
to
834b2c9
Compare
Rebased on latest master |
e06f779
to
5f90af0
Compare
Rebased on latest master |
85a9521
to
3965bef
Compare
Rebased on latest master |
tests/conftest.py
Outdated
(MultiClusterUpgradeParametrize.MULTICLUSTER_UPGRADE_MARKERS) | ||
) | ||
) | ||
and ocsci_config.multicluster_scenario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to ocsci_config.multicluster
ocs_ci/ocs/dr_upgrade.py
Outdated
self.pre_upgrade_images = self.get_pre_upgrade_image(self.csv_name_pre_upgrade) | ||
self.load_version_config_file(self.upgrade_version) | ||
|
||
with CephHealthMonitor(self.ceph_cluster): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to instantiate self.ceph_cluster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest dropping out the ceph health monitor for now, as this will require more adjustment in the ceph cluster object and also in the health monitor context manager to enforce it to run on specific cluster and propagate specific kubeconfig for all of underlaying calls.
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
…lass Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
1. Catsrc name patching issue 2. Format icsp yaml 3. patch channel name Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
rebased on the latest master |
self.acm_registry_image = config.UPGRADE.get("upgrade_acm_registry_image", "") | ||
self.zstream_upgrade = False | ||
|
||
def get_acm_version_before_upgrade(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity, what is the reason to not use directly get_running_acm_version
and instead use this wrapper?
I'm not sure, if there might be a situation, where we would like to call this method on other stages of the workflow, but if it will be called after the upgrade, it will return the actual upgraded version, not the version before upgrade as suggested by the name. Or did I miss anything there?
(MultiClusterUpgradeParametrize.MULTICLUSTER_UPGRADE_MARKERS) | ||
) | ||
) | ||
and ocsci_config.multicluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part of the condition and ocsci_config.multicluster
is probably not necessary here as it is also part of the condition on line 479:
if ocsci_config.multicluster and ocsci_config.UPGRADE.get("upgrade", False):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just two questions mentioned above (but they are more questions or notes, not thinks which have to be necessarily fixed).
I don't have so deep knowledge in the existing upgrade automation, so I'm not able to easily distinguish some possible technical gaps if there are any, so overall LGTM.
Signed-off-by: Shylesh Kumar Mohan <[email protected]>
New changes are detected. LGTM label has been removed. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shylesh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shylesh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR addresses the following